Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Review SummaryThis PR consolidates redundant Critical — will crash at runtime1. 2. 3.
All three need 4. Confirmed good fixes in this PR
The alias consolidation approach is correct; it just needs the four call sites above updated before merging. |
There was a problem hiding this comment.
Pull request overview
This PR appears to follow up on review feedback from #1247 by standardizing schema-related error codes/messages, cleaning up dead/commented code, and making a few small validator/analysis fixes.
Changes:
- Consolidates multiple schema/header/library error codes into fewer canonical
HedExceptionscodes and updates tests accordingly. - Refactors/cleans up validators and tools (e.g., precompiled column-ref regex, removes unused
make_path, removes commented code). - Fixes a few correctness issues in analysis/utilities (e.g., tag count merge bug, onset tolerance constant, error message typos).
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/schema/test_schema_wiki_fatal_errors.py | Updates expected fatal error codes for wiki schema header cases. |
| tests/schema/test_hed_schema_io.py | Updates prerelease-partner tests/docs to assert the new error code. |
| spec_tests/test_hed_cache.py | Updates expected exception code for invalid merged schema load cases. |
| hed/validator/util/group_util.py | Removes unused commented-out validator hooks. |
| hed/validator/spreadsheet_validator.py | Fixes onset tolerance constant to a small float tolerance. |
| hed/validator/sidecar_validator.py | Precompiles the column reference regex and reuses it. |
| hed/validator/def_validator.py | Removes commented-out unit validation code. |
| hed/tools/util/io_util.py | Removes the unused make_path helper. |
| hed/tools/bids/bids_dataset.py | Fixes an error message typo (extra }). |
| hed/tools/analysis/hed_type_factors.py | Normalizes factor encoding errors to raise HedFileError and fixes a doc typo. |
| hed/tools/analysis/hed_tag_counts.py | Fixes merge logic when summing per-value counts. |
| hed/tools/analysis/annotation_util.py | Removes large blocks of commented-out legacy helpers. |
| hed/tools/init.py | Stops exporting make_path (aligned with its removal). |
| hed/schema/schema_io/wiki2schema.py | Uses SCHEMA_HEADER_INVALID for missing/invalid header line. |
| hed/schema/schema_io/base2schema.py | Adjusts error codes for schema merge/load and rooted-tag validation paths. |
| hed/schema/schema_header_util.py | Improves library-name validation messages and aligns error codes. |
| hed/schema/hed_schema_group.py | Uses SCHEMA_LOAD_FAILED for duplicate namespace/prefix collisions. |
| hed/errors/exceptions.py | Removes legacy/alias exception constants in favor of canonical codes. |
| hed/errors/error_messages.py | Fixes missing trailing quote in formatted def/def-expand messages. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
|
PR Review summary: see inline comments. Critical fixes verified: ONSET_TOLERANCE (was 3, now 1e-7), dict+int TypeError in merge_tag_dicts, unclosed f-string quotes, wrong exception type in hed_type_factors. One important issue flagged via inline comment on exceptions.py (public API break from removing HedExceptions aliases). One suggestion flagged via inline comment on test_hed_schema_io.py (misleading comment references non-existent LIBRARY_SCHEMA_INVALID constant). |
No description provided.